Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Reauthenticate components to handle generic MFA challenges. #49680

Merged
merged 13 commits into from
Dec 17, 2024

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Dec 3, 2024

The overarching change in this PR is the ability of all Reauthenticate flows to handle generic MFA challenges (sso, webauthn, otp) rather than handling each type differently or not at all.

Note: useReauthenticate should now be the goto state handler method for reauthentication flows.

Changes:

  • Refactor useReauthenticate (used in the Reauthenticate components).
    • Return a submitWithMFA method instead of submitWithWebAuthn and submitWithOTP. SSO MFA will use submitWithMFA('sso').
    • Deprecate OnAuthenticated prop
    • Hold MFA challenge in state - prevents the current behavior of retrieving a new MFA challenge every time you open the component or switch between available options. We only need a fresh MFA challenge after the current challenge has been used up.
    • Use update MFA Options from WebUI MFA types refactor #49678 to derive reauth and register options from MFA challenge ^
  • Update change/add device wizards according to useReauthenticate refactors.
  • Use useReauthenticate for change password wizard to get the same changes described above.
  • Update createPrivilegeToken endpoint to accept generic MFA response instead of TOTP or Webauthn, so it will now support SSO MFA.

Note on /e: This PR is dependent on the e PR and vice versa. Therefore I'll need to merge this PR with e PR as the head of /e. Then I'll make a follow up to update the e ref to master once the e PR is merged, shortly after this PR is merged.

TODO (follow ups):

  • (Optional) Instead of getting a privilege token before adding/deleting a device, just use the mfa response directly.
    • This is a piece of work which has been forgotten after being made possible with Let authenticated users issue register challenges #32271
    • Note to self: we will still need to get a privilege token if the user clicks the otp register option since it would cash in the mfa response for the qr code, and as a result the user would need to reauthenticate to switch to webauthn. Also failure/cancel on the last step may need to go back to initial reauth step.

Prerequisite for supporting SSO MFA in device management (add/delete), connection tester (discover), change password, and account recovery flows.

Depends on #49678, #49679, https://github.com/gravitational/teleport.e/pull/5656

@Joerger Joerger force-pushed the joerger/unify-mfa-methods branch from 0d9623a to 518f3ef Compare December 3, 2024 20:59
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from 9d5b285 to b4e63c1 Compare December 3, 2024 20:59
@Joerger Joerger force-pushed the joerger/unify-mfa-methods branch 3 times, most recently from e2afc8c to e53c97c Compare December 3, 2024 23:40
@Joerger Joerger mentioned this pull request Dec 4, 2024
@Joerger Joerger force-pushed the joerger/unify-mfa-methods branch from e53c97c to a36af16 Compare December 4, 2024 20:46
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from b4e63c1 to 5eca740 Compare December 4, 2024 21:09
@Joerger Joerger marked this pull request as ready for review December 4, 2024 21:09
@Joerger Joerger requested a review from avatus December 4, 2024 21:09
@Joerger Joerger force-pushed the joerger/unify-mfa-methods branch from a36af16 to 037f575 Compare December 4, 2024 21:27
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch 4 times, most recently from 4abf873 to 42ba430 Compare December 6, 2024 00:11
@Joerger Joerger added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 backport/branch/v16 backport/branch/v17 and removed backport/branch/v15 labels Dec 6, 2024
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from ea8e8eb to 40336ed Compare December 9, 2024 22:26
@Joerger Joerger force-pushed the joerger/unify-mfa-methods branch from ad04916 to 831528d Compare December 9, 2024 22:29
Base automatically changed from joerger/unify-mfa-methods to master December 10, 2024 00:51
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch 2 times, most recently from af41a4e to bdd69ca Compare December 10, 2024 01:04
@Joerger
Copy link
Contributor Author

Joerger commented Dec 10, 2024

@ryanclark @avatus I added one more commit if you don't mind taking a look e60ec3d

@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch 4 times, most recently from 828bded to 5d0f08d Compare December 14, 2024 01:50
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from 5d0f08d to 89e4639 Compare December 16, 2024 22:17
@Joerger Joerger added this pull request to the merge queue Dec 17, 2024
Merged via the queue into master with commit 0f463c5 Dec 17, 2024
45 checks passed
@Joerger Joerger deleted the joerger/refactor-reauthenticate branch December 17, 2024 03:45
@public-teleport-github-review-bot

@Joerger See the table below for backport results.

Branch Result
branch/v17 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants